Skip to content

Show error message when running query on empty list#1860

Merged
norascheuch merged 3 commits intomainfrom
nora/show-error-empty-list
Dec 13, 2022
Merged

Show error message when running query on empty list#1860
norascheuch merged 3 commits intomainfrom
nora/show-error-empty-list

Conversation

@norascheuch
Copy link
Copy Markdown
Contributor

When creating a new DB List in the panel it will be empty at first. We are now showing an error if one tries to run a query on an empty remote DB list.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@norascheuch norascheuch marked this pull request as ready for review December 13, 2022 10:23
@norascheuch norascheuch requested a review from a team as a code owner December 13, 2022 10:23
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random drive-by typo fix, but I'll let the rest of the people working on this feature review the rest.

…ies/repository-selection.test.ts

Co-authored-by: Robert <robertbrignull@github.com>
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speedy ⚡

Generally looking good, just some small comments.

repositories: selectedDbItem.repos.map((repo) => repo.repoFullName),
};
if (selectedDbItem.repos.length === 0) {
throw new Error(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to do showAndLogErrorMessage rather than throwing an exception. It has the same outcome from a user's perspective but it won't be considered as an unhandled error from our app's perspective.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I changed the other errors in that switch as well!

Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts Outdated
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

Comment thread extensions/ql-vscode/src/remote-queries/repository-selection.ts Outdated
@norascheuch norascheuch force-pushed the nora/show-error-empty-list branch from cfb6cf1 to 0f1881d Compare December 13, 2022 15:14
@norascheuch norascheuch merged commit b228549 into main Dec 13, 2022
@norascheuch norascheuch deleted the nora/show-error-empty-list branch December 13, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants